Remove legacy doctrine annotation comments from provider classes #34
Conversation
This change addresses issue #33 by removing legacy doctrine annotation comments (@RedisConfig, @MemcacheConfig) from provider class docblocks. After migrating to PHP 8 attributes in version 1.5.0, these legacy annotations were causing AnnotationException when packages using koriym/attributes' DualReader attempted to parse them. Changes: - src/RedisProvider.php: Remove @RedisConfig("server") comment - src/MemcachedProvider.php: Remove @MemcacheConfig("servers") comment The PHP 8 attributes remain intact and functional. Type hints (@param) and documentation comments (@see) are preserved. Fixes #33
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR removes outdated doctrine annotation comments (@RedisConfig, @MemcacheConfig) from provider class docblocks to prevent AnnotationException in DualReader, while retaining PHP 8 attributes and other docblock tags. Class diagram for updated provider constructors (RedisProvider, MemcachedProvider)classDiagram
class RedisProvider {
+__construct(array server)
<<attribute>> RedisConfig('server')
}
class MemcachedProvider {
+__construct(array servers)
<<attribute>> MemcacheConfig('servers')
}
ProviderInterface <|.. RedisProvider
ProviderInterface <|.. MemcachedProvider
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #34 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 38 38
===========================================
Files 24 24
Lines 90 90
===========================================
Hits 90 90 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughRemoved legacy doctrine annotation comments ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider doing a global search for any remaining legacy Doctrine annotation comments in other classes to prevent similar
AnnotationExceptionissues. - For consistency, you might standardize the docblock formatting between RedisProvider and MemcachedProvider (e.g., single-line vs multi-line) now that the old annotations are removed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider doing a global search for any remaining legacy Doctrine annotation comments in other classes to prevent similar `AnnotationException` issues.
- For consistency, you might standardize the docblock formatting between RedisProvider and MemcachedProvider (e.g., single-line vs multi-line) now that the old annotations are removed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@Sourcely I've verified both points: Global search for legacy annotationsI performed a comprehensive search across the grep -r "@(RedisConfig|MemcacheConfig|CacheDir|CacheNamespace)" src/No other legacy Doctrine annotation comments were found. Only the two files in this PR contained them. Docblock formatting consistencyThe current docblock formatting is already consistent and follows PSR-12 standards:
This difference is intentional and semantic. Both formats comply with our coding standards (enforced by phpcbf). If we were to force both to use the same format, we would either:
The current approach follows the principle of using multi-line docblocks only when there's multiple types of documentation to include. |
Summary
Removes legacy doctrine annotation comments from provider classes to fix
AnnotationExceptionwhen usingkoriym/attributeswithDualReader.Changes
@RedisConfig("server")annotation comment@MemcacheConfig("servers")annotation commentDetails
After migrating to PHP 8 attributes in version 1.5.0, legacy doctrine annotation comments remained in provider class docblocks. These cause
AnnotationExceptionwhen downstream packages usingkoriym/attributes'sDualReaderattempt to parse them.The issue occurs because:
@RedisConfigcomments and new#[RedisConfig]attributesDualReaderfinds@RedisConfigand tries to process it as a doctrine annotationRedisConfigclass no longer has@Annotationdocblock (correctly removed during PHP 8 migration)AnnotationReaderthrowsAnnotationExceptionWhat's Preserved
#[RedisConfig],#[MemcacheConfig])@param,@return,@var)@see,@throws)Testing
composer cs-fixapplied successfullyFixes #33
Summary by Sourcery
Remove legacy Doctrine annotation comments from provider classes to prevent AnnotationException when using DualReader.
Bug Fixes:
Summary by CodeRabbit
Refactor